-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
server: use no-op cost controller for shared-process tenants #99113
server: use no-op cost controller for shared-process tenants #99113
Conversation
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @stevendanna)
pkg/server/tenant.go
line 194 at r1 (raw file):
// the tenant is exempt. For now we are turning off the // tenant-side cost controller for shared-memory tenants until // we have the abilility to read capabilities tenant-side.
Can you file the followup issue and link it from here thanks
In the long run, all rate limiting of tenants should be controlled by a tenant capability. However, at the moment, we do not have the infrastructure to read tenant capabilities from the tenant process. Since, the main user of shared-process tenants is the new, experimental unified-architecture were we do not anticipate needing the cost controller for most users, this PR injects a no-op cost controller for shared-process tenants. Epic: CRDB-23559 Release note: None
This exempts the tenant from the KV-side tenant limiter. We expect this to be the default configuration of most shared-process tenants in the near term. Thus, setting this during benchmark setups provides more accurate benchmarks. Epic: none Release note: None
b2f9558
to
9e05d17
Compare
bors r=knz |
Build failed (retrying...): |
Build succeeded: |
In the long run, all rate limiting of tenants should be controlled by
a tenant capability.
However, at the moment, we do not have the infrastructure to read
tenant capabilities from the tenant process.
Since, the main user of shared-process tenants is the new,
experimental unified-architecture were we do not anticipate needing
the cost controller for most users, this PR injects a no-op cost
controller for shared-process tenants.
Epic: CRDB-23559
Release note: None